Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add: Added a dedicated view for compliance audit reports #3952

Merged
merged 37 commits into from
Aug 23, 2024

Conversation

a-h-abdelsalam
Copy link
Contributor

@a-h-abdelsalam a-h-abdelsalam commented Jan 25, 2024

What

  • Added a dedicated view for compliance audit reports. Compliance reports are now listed under Resilience tab and do not appear anymore under Scans tab. A dedicated view for a compliance report shows compliance of results.
  • Delta compliance reports can highlight changes in compliance.
  • Report links in Audits page now link to audit reports view.
  • This feature is currently behind a feature toggle: COMPLIANCE_REPORTS

Why

Currently GSA doesn't distinguish between different types of reports (vulnerability, compliance audit, etc.). Therefore there is no dedicated view yet, causing all filters to be accessible in different reports.

References

GEA-397, GEA-613
requires greenbone/gvmd#2125
requires greenbone/gsad#164

  • Tests

Copy link

Conventional Commits Report

Type Number
Added 1

🚀 Conventional commits found.

Copy link

codecov bot commented Jan 25, 2024

Codecov Report

Attention: Patch coverage is 74.09390% with 1258 lines in your changes missing coverage. Please review.

Project coverage is 72.85%. Comparing base (5a78a99) to head (f0e67ed).

Current head f0e67ed differs from pull request most recent head 7be3325

Please upload reports for the commit 7be3325 to get more accurate results.

Files Patch % Lines
src/web/pages/reports/auditdetailspage.jsx 17.69% 572 Missing ⚠️
src/web/pages/reports/auditdeltadetailspage.jsx 20.61% 439 Missing ⚠️
src/gmp/commands/auditreports.js 67.37% 46 Missing ⚠️
src/web/pages/reports/auditreportslistpage.jsx 84.11% 27 Missing ⚠️
src/web/pages/reports/details/resultstab.jsx 57.62% 25 Missing ⚠️
.../web/pages/reports/details/auditthresholdpanel.jsx 83.78% 24 Missing ⚠️
src/web/pages/reports/auditdetailscontent.jsx 95.10% 20 Missing and 2 partials ⚠️
src/web/pages/reports/auditreportrow.jsx 87.83% 18 Missing ⚠️
src/web/pages/usersettings/filterpart.jsx 7.69% 12 Missing ⚠️
src/web/pages/usersettings/dialog.jsx 8.33% 11 Missing ⚠️
... and 15 more
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3952      +/-   ##
==========================================
+ Coverage   72.65%   72.85%   +0.20%     
==========================================
  Files        1062     1078      +16     
  Lines      120912   125058    +4146     
  Branches     5990     6206     +216     
==========================================
+ Hits        87843    91114    +3271     
- Misses      33040    33913     +873     
- Partials       29       31       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@a-h-abdelsalam a-h-abdelsalam force-pushed the add-dedicated-view-for-compliance-reports branch from dcd49bf to 0fa51d0 Compare February 2, 2024 15:30
@a-h-abdelsalam a-h-abdelsalam force-pushed the add-dedicated-view-for-compliance-reports branch from eb4a4af to b717560 Compare February 13, 2024 13:28
@a-h-abdelsalam a-h-abdelsalam marked this pull request as ready for review February 14, 2024 15:45
@a-h-abdelsalam a-h-abdelsalam requested a review from a team as a code owner February 14, 2024 15:45
@a-h-abdelsalam a-h-abdelsalam force-pushed the add-dedicated-view-for-compliance-reports branch from f2a6c69 to 811e5a9 Compare February 20, 2024 10:52
Copy link
Contributor

@bjoernricks bjoernricks left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

First of all this PR looks good. My comments are mostly only style and convention related.

Some rules that should be considered:

  • In JS we agreed on camelCase usage for functions, classes, variables and properties finally. Because I have a Python background and wrote most of the initial code we still have a lot of snake_case naming which we never get rid of. All new additions should use camelCase when possible. Personally I am not sure if this fits here with the report still using snake_case and the strong coupling between the audit, tasks, scan configs, policies and the two different reports.
  • We agreed on using absolute imports without the .js suffix some while ago. This might not be consistent on all places.
  • New react components should be function components if possible. React class components are still supported and will be supported a long time but are considered legacy. And because you can't use hooks inside class components it is better to write only function components for new code.

src/gmp/commands/__tests__/auditreport.js Outdated Show resolved Hide resolved
src/gmp/models/report/auditreport.js Outdated Show resolved Hide resolved
src/gmp/models/report/auditreport.js Outdated Show resolved Hide resolved
src/gmp/models/report/auditreport.js Show resolved Hide resolved
src/gmp/models/report/auditreport.js Show resolved Hide resolved
src/web/pages/tasks/row.js Outdated Show resolved Hide resolved
src/web/pages/reports/auditdetailscontent.js Outdated Show resolved Hide resolved
src/web/pages/reports/auditdetailscontent.js Outdated Show resolved Hide resolved
src/web/pages/reports/auditdetailscontent.js Outdated Show resolved Hide resolved
src/web/pages/reports/auditdetailscontent.js Outdated Show resolved Hide resolved
@a-h-abdelsalam
Copy link
Contributor Author

Thank you for your review @bjoernricks. I applied the review suggestions where possible and also rewrote auditdeltadetailspage, auditdetailspage, auditreportlistpage, compliancelevelsgroup into functional components.

@a-h-abdelsalam a-h-abdelsalam force-pushed the add-dedicated-view-for-compliance-reports branch from 83a22c9 to 8a16fa8 Compare March 7, 2024 14:14
src/web/components/powerfilter/compliancelevelsgroup.js Outdated Show resolved Hide resolved
src/web/components/powerfilter/compliancelevelsgroup.js Outdated Show resolved Hide resolved
src/web/components/powerfilter/compliancelevelsgroup.js Outdated Show resolved Hide resolved
src/web/pages/reports/auditreportslistpage.js Outdated Show resolved Hide resolved
src/web/pages/reports/auditreportslistpage.js Outdated Show resolved Hide resolved
src/web/pages/reports/auditreportslistpage.js Outdated Show resolved Hide resolved
src/web/pages/reports/auditdeltadetailspage.js Outdated Show resolved Hide resolved
src/web/pages/reports/auditdeltadetailspage.js Outdated Show resolved Hide resolved
src/web/pages/reports/auditdeltadetailspage.js Outdated Show resolved Hide resolved
src/web/pages/reports/auditdetailspage.js Outdated Show resolved Hide resolved
@a-h-abdelsalam a-h-abdelsalam marked this pull request as draft March 12, 2024 14:58
@a-h-abdelsalam a-h-abdelsalam force-pushed the add-dedicated-view-for-compliance-reports branch from a4a9c06 to b474908 Compare April 4, 2024 16:18
@a-h-abdelsalam a-h-abdelsalam marked this pull request as ready for review April 4, 2024 16:53
@a-h-abdelsalam a-h-abdelsalam marked this pull request as draft April 5, 2024 07:58
@a-h-abdelsalam a-h-abdelsalam force-pushed the add-dedicated-view-for-compliance-reports branch from b474908 to 349acd0 Compare May 13, 2024 12:28
@a-h-abdelsalam a-h-abdelsalam force-pushed the add-dedicated-view-for-compliance-reports branch from c039b3e to ae14f69 Compare June 11, 2024 12:29
Copy link

github-actions bot commented Jun 11, 2024

Dependency Review

✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.

Snapshot Warnings

⚠️: No snapshots were found for the head SHA 9ce7ec5.
Ensure that dependencies are being submitted on PR branches and consider enabling retry-on-snapshot-warnings. See the documentation for more information and troubleshooting advice.

OpenSSF Scorecard

PackageVersionScoreDetails

Scanned Manifest Files

@a-h-abdelsalam a-h-abdelsalam force-pushed the add-dedicated-view-for-compliance-reports branch 2 times, most recently from 520fa21 to 32096cc Compare June 12, 2024 08:58
@a-h-abdelsalam a-h-abdelsalam marked this pull request as ready for review June 12, 2024 10:16
@a-h-abdelsalam a-h-abdelsalam force-pushed the add-dedicated-view-for-compliance-reports branch from 32096cc to c83f4ae Compare June 12, 2024 14:09
src/gmp/models/auditreport.js Outdated Show resolved Hide resolved
src/web/components/bar/compliancebar.jsx Outdated Show resolved Hide resolved
src/web/components/label/compliancestate.jsx Outdated Show resolved Hide resolved
src/web/components/powerfilter/compliancelevelsgroup.jsx Outdated Show resolved Hide resolved
src/web/components/powerfilter/compliancelevelsgroup.jsx Outdated Show resolved Hide resolved
src/web/pages/reports/auditreportslistpage.jsx Outdated Show resolved Hide resolved
src/web/pages/reports/auditreportslistpage.jsx Outdated Show resolved Hide resolved
src/web/pages/reports/auditreportstable.jsx Show resolved Hide resolved
src/web/pages/reports/auditreportstable.jsx Outdated Show resolved Hide resolved
src/web/pages/reports/__tests__/auditfilterdialog.jsx Outdated Show resolved Hide resolved
@a-h-abdelsalam a-h-abdelsalam marked this pull request as draft June 13, 2024 15:12
- Rewrite auditdeltadetailspage, auditdetailspage, auditreportlistpage,
  compliancelevelsgroup into functional components.
- Use camel case instead of snake case
- Use absolute imports without .js suffix
Rename *.js to *.jsx and make sure tests are running again.
Report links should switch to report or compliance report details
page based on the feature toggle.
@a-h-abdelsalam a-h-abdelsalam force-pushed the add-dedicated-view-for-compliance-reports branch from f0f0047 to 9ce7ec5 Compare August 23, 2024 08:34
@a-h-abdelsalam a-h-abdelsalam merged commit 52940b5 into main Aug 23, 2024
14 of 15 checks passed
@a-h-abdelsalam a-h-abdelsalam deleted the add-dedicated-view-for-compliance-reports branch August 23, 2024 13:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants